fix: allow zero-voter distributions (#77)#121
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates distribution readiness rules to allow MultiStrategyDistributionManager distributions to proceed even when there are zero active votes, enabling fixed-grant style distributions for communities with low or no voter participation.
Changes:
- Removes the
totalVotes == 0readiness gate fromMultiStrategyDistributionManager.isDistributionReady(). - Updates NatSpec around
DistributionNotReady/ readiness expectations to reflect the new behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/interfaces/IDistributionManager.sol |
Updates NatSpec for DistributionNotReady to reflect revised readiness assumptions. |
src/base/MultiStrategyDistributionManager.sol |
Changes readiness logic to permit zero-voter distributions and updates function NatSpec accordingly. |
Comments suppressed due to low confidence (1)
src/base/MultiStrategyDistributionManager.sol:104
- This PR changes readiness semantics (zero-voter distributions now allowed), but there doesn't appear to be a test asserting
MultiStrategyDistributionManager.isDistributionReady()returns true when votes are 0 and other conditions are satisfied. Add a unit/integration test covering this case (and, ideally, a case showing vote-dependent strategies still block distribution appropriately if you keep that behavior).
function isDistributionReady() public view override returns (bool ready) {
if (cycleManager().distributionManager() != address(this)) return false;
if (!cycleManager().isCycleComplete()) return false;
// Allow zero-voter distributions — small communities may legitimately have no votes
// but still want to distribute to recipients (e.g., fixed grants).
uint256 recipientCount = recipientRegistry().getRecipientCount();
if (recipientCount == 0) return false;
MultiStrategyDistributionManagerStorage storage $ = _getMultiStrategyDistributionManagerStorage();
uint256 strategyCount = $.strategies.length;
if (strategyCount == 0) return false;
uint256 yieldAmount = yieldModule().yieldAccrued();
if (yieldAmount == 0) return false;
// Require enough yield so that, after equal split across strategies,
// each strategy can distribute at least one unit per recipient.
uint256 minRequiredYield = recipientCount * strategyCount;
return yieldAmount >= minRequiredYield;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// @dev Distribution is not ready when cycle is incomplete, there are no recipients, | ||
| /// or yield is insufficient. Note: MultiStrategyDistributionManager allows | ||
| /// zero-voter distributions for fixed-grant use cases. |
There was a problem hiding this comment.
The devdoc on DistributionNotReady describes readiness conditions as if they were universal, but BaseDistributionManager.isDistributionReady() still returns false when total voting power is 0. Consider making this comment implementation-agnostic (e.g., “conditions vary by implementation; common reasons include …”) and move the MultiStrategy-specific note to MultiStrategyDistributionManager docs.
| /// @dev Distribution is not ready when cycle is incomplete, there are no recipients, | |
| /// or yield is insufficient. Note: MultiStrategyDistributionManager allows | |
| /// zero-voter distributions for fixed-grant use cases. | |
| /// @dev Readiness conditions vary by implementation. Common reasons include an | |
| /// incomplete cycle, no eligible recipients, insufficient available yield, | |
| /// or other implementation-specific requirements not being satisfied. |
| /// @notice Checks if distribution is ready based on cycle completion, votes, recipients, strategies, and yield | ||
| /// @return ready True if cycle is complete, there are votes, recipients, configured strategies, and sufficient yield | ||
| /// @return ready True if cycle is complete, there are recipients, configured strategies, and sufficient yield | ||
| /// @dev Allows zero-voter distributions for small communities (matches breadchain contracts) |
There was a problem hiding this comment.
isDistributionReady() no longer checks voting power, but the NatSpec @notice still says readiness is based on “votes”. Update the notice to match the actual criteria (cycle complete, recipients/strategies configured, sufficient yield).
| function isDistributionReady() public view override returns (bool ready) { | ||
| if (cycleManager().distributionManager() != address(this)) return false; | ||
| if (!cycleManager().isCycleComplete()) return false; | ||
|
|
||
| uint256 totalVotes = getTotalCurrentVotingPower(); | ||
| if (totalVotes == 0) return false; | ||
|
|
||
| // Allow zero-voter distributions — small communities may legitimately have no votes | ||
| // but still want to distribute to recipients (e.g., fixed grants). | ||
| uint256 recipientCount = recipientRegistry().getRecipientCount(); | ||
| if (recipientCount == 0) return false; | ||
|
|
There was a problem hiding this comment.
With the totalVotes == 0 gate removed, isDistributionReady() can return true even when configured strategies will revert on zero votes (e.g., VotingDistributionStrategy reverts with NoVotes). This can cause automation to repeatedly attempt claimAndDistribute() and revert. Consider either (a) keeping a conservative zero-votes check unless the manager is explicitly configured to allow it, or (b) introducing a strategy-level readiness signal/interface so vote-dependent strategies can block readiness.
RonTuretzky
force-pushed
the
RonTuretzky/64-fuzz-tests
branch
from
e21f9c5 to
a5b0cd1
Compare
Remove the totalVotes == 0 early-return from isDistributionReady() so small communities with no active voters can still distribute yield to recipients (e.g., fixed grants). Matches breadchain contract behavior.
RonTuretzky
force-pushed
the
RonTuretzky/104-zero-voter
branch
from
c43ae09 to
4e3c4a2
Compare
RonTuretzky
left a comment
RonTuretzky
left a comment
There was a problem hiding this comment.
Remove breadchain ref
Summary
totalVotes == 0early return fromMultiStrategyDistributionManager.isDistributionReady()Closes #77
Supersedes #104
Stack: PR 4 of 10 (0.0.2) — stacked on #120